Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix mesh / topo intersection #1062

Merged
merged 3 commits into from
Nov 22, 2023
Merged

fix mesh / topo intersection #1062

merged 3 commits into from
Nov 22, 2023

Conversation

andrewheumann
Copy link
Member

@andrewheumann andrewheumann commented Nov 21, 2023 •
edited
Loading

BACKGROUND:

  • Mesh/Ray and Topo/Ray intersection would return the wrong point for closed volumes — it was only returning the first intersection, rather than the nearest, so it was impossible to (e.g.) project a ray down onto a topography and get the location of the intersection.
  • The RayIntersectTopo test got blown away by this PR with what appears to be some temporary code — it switched from testing Ray.Intersects(Topography) to testing Ray.Intersects(Triangle).
    image

DESCRIPTION:

  • Checks all near triangles, rather than aborting after the first one.
  • Fixes the RayIntersectsTopo test

REQUIRED:

  • All changes are up to date in CHANGELOG.md.

This change is Reviewable

jamesbradleym
jamesbradleym approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@jamesbradleym jamesbradleym left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! It probably makes sense for the RayIntersectsTopography test to use ray.Intersects(topo, out var result) rather than ray.Intersects(topo.Mesh, out var result)

One question I have regarding the Intersects method that uses a Topo. Presumably if I'm looking for the intersection of a topo (and MeshElements with a Transform in general) I'd expect that intersection point to be where the actual geometry is rather than where the untransformed mesh is.

I'm not sure it makes sense necessarily to have this method on topo if it takes doesn't adjust to the topo.Transform.Matrix.Translation?

Approving incase it's expected to receive the untransformed intersection but this seems unexpected to me.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@andrewheumann andrewheumann merged commit a4f0d19 into master Nov 22, 2023
1 check passed
@andrewheumann andrewheumann deleted the fix-mesh-intersection branch November 22, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesbradleym jamesbradleym jamesbradleym approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrewheumann @jamesbradleym